Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TS impl #1

Closed
wants to merge 17 commits into from
Closed

TS impl #1

wants to merge 17 commits into from

Conversation

decentralgabe
Copy link
Member

@decentralgabe decentralgabe commented Oct 3, 2023

So far:

  • structure
  • did creation
  • key generation
  • compression
  • tests
  • dht publish and read / en/decoding to DID documents

@decentralgabe decentralgabe marked this pull request as ready for review October 4, 2023 22:04
Copy link

@kirahsapong kirahsapong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice!! some comments or q's, prob mostly nits

impls/ts/src/did-dht.ts Show resolved Hide resolved
impls/ts/src/did-dht.ts Outdated Show resolved Hide resolved
impls/ts/src/did-dht.ts Outdated Show resolved Hide resolved
impls/ts/src/did-dht.ts Outdated Show resolved Hide resolved
impls/ts/src/did-dht.ts Outdated Show resolved Hide resolved
impls/ts/src/did-dht.ts Outdated Show resolved Hide resolved
impls/ts/src/did-dht.ts Outdated Show resolved Hide resolved
impls/ts/tests/did-dht.spec.ts Outdated Show resolved Hide resolved
impls/ts/tests/did-dht.spec.ts Outdated Show resolved Hide resolved
impls/ts/tests/did-dht.spec.ts Outdated Show resolved Hide resolved
@michaelneale
Copy link

exciting to see!

@Nuhvi
Copy link

Nuhvi commented Oct 6, 2023

Not sticking to DNS packet codec is a mistake. You should take my word for it because the codec you are using is already what I started with and moved from[1].

DIDs are awful anyway, but if you have to do it, put in a TXT record[2], giving up on DNS semantics for DIDs is just as unwise as all the time and money spent on ION despite my repeated advice against it.

[1] Nuhvi/pkarr@4fd7e71
[2] https://gist.github.com/Nuhvi/443b7dc066b6f5e9236adfb99b241939

@Nuhvi
Copy link

Nuhvi commented Oct 6, 2023

We know for a fact that domains are useful and pretty much the closest thing to a source of sovereignty in the web architecture, taking it to the next level with TLDs that are self-issued and censorship-resistant is useful regardless of what data you put in it.

From TTLs, to the wide support in every browser and operating system to the fact that DNS Packets simply do "service endpoints" better with A, AAAA, and CNAME, and that is infinitely more supported than DIDs, it is irresponsible to consciously decide to forgo all of that for no reason other than NIH, especially when the NIH is one of the pillars of the Internet!

@mistermoe
Copy link
Contributor

@decentralgabe pushed a commit that does a bit of housekeeping and added a github workflow to run tests.

a few questions before reviewing the code itself:

  • would it make more sense to add this to the @web5/dids package? that's where our js implementations of did:key and did:ion currently live and where we plan to introduce additional did method support (e.g. did:jwk, did:mic). Also it'd ensure that the implementation works with the rest of our pre-existing web5 functionality
  • what runtimes are you initially targeting? just node? node and browser? node, browser, and react native?

@decentralgabe
Copy link
Member Author

@Nuhvi our starting point doesn't exclude the possibility of moving to DNS semantics, just we won't get much benefit from them out of the gate.

Closing this in favor of moving to web5-js.

@Nuhvi
Copy link

Nuhvi commented Oct 6, 2023

@decentralgabe A big benefit you will get out of the gate, is using Pkarr's implementations in JS and in Rust, and knowing that they follow best practices and have a certain performance profile.

You are betting that implementing a good DHT client is easier and faster than conforming to DNS resource records.

@decentralgabe decentralgabe deleted the ts-impl branch October 6, 2023 19:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants